[손수진] Sprint4 #143
[손수진] Sprint4 #143
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
수진님 4번째 미션 제출 고생하셨습니다!
공통화하려고 노력하신 것이 보여서 좋았어요~
관련해서 피드백 드렸으니 참고해보시면 좋겠습니다.
다음 React 미션도 화이팅입니다.
| <script src="./login.js" type="module"></script> | ||
| <script src="./loginFunctions.js" type="module"></script> |
There was a problem hiding this comment.
💊 제안
script 태그는 상단에 있는게 구조 파악측면에서 유리하다고 생각해서 큰 이유가 없다면 상단 head 태그에 두시는 것을 추천드려요~
특히 타입이 모듈인 스크립트는 defer 속성을 자동으로 가지기 때문에 따로 하단에 배치하실 이유가 없습니다~
| const ERRORMESSAGE = { | ||
| wrongEmail: "잘못된 이메일 형식입니다", | ||
| wrongPassword: "비밀번호를 8자 이상 입력해주세요", | ||
| emailIsEmpty: "이메일을 입력해주세요", | ||
| passwordIsEmpty: "비밀번호를 입력해주세요", | ||
| nicknameIsEmpty: "닉네임을 입력해주세요", | ||
| passwordIsNotSame: "비밀번호가 일치하지 않습니다.", | ||
| }; |
There was a problem hiding this comment.
💊 제안
반복사용되는 에러메시지를 따로 상수로 모아두신 것 좋습니다. 다만 단어를 구분할 수 있게 아래처럼 명명하시는 것을 추천드려요!
| const ERRORMESSAGE = { | |
| wrongEmail: "잘못된 이메일 형식입니다", | |
| wrongPassword: "비밀번호를 8자 이상 입력해주세요", | |
| emailIsEmpty: "이메일을 입력해주세요", | |
| passwordIsEmpty: "비밀번호를 입력해주세요", | |
| nicknameIsEmpty: "닉네임을 입력해주세요", | |
| passwordIsNotSame: "비밀번호가 일치하지 않습니다.", | |
| }; | |
| const ERROR_MESSAGE = { | |
| wrongEmail: "잘못된 이메일 형식입니다", | |
| wrongPassword: "비밀번호를 8자 이상 입력해주세요", | |
| emailIsEmpty: "이메일을 입력해주세요", | |
| passwordIsEmpty: "비밀번호를 입력해주세요", | |
| nicknameIsEmpty: "닉네임을 입력해주세요", | |
| passwordIsNotSame: "비밀번호가 일치하지 않습니다.", | |
| }; |
| } | ||
| }; | ||
|
|
||
| const createNewMessage = (messageContent, target) => { |
There was a problem hiding this comment.
💊 제안
에러 메시지를 보여주는 것이니 이를 알 수 있는 이름으로 하시는 것을 추천드립니다~
| const createNewMessage = (messageContent, target) => { | |
| const createErrorMessage = (messageContent, target) => { |
There was a problem hiding this comment.
💊 제안
내부 로직들을 loginFunction으로 분리하신점 좋습니다~
다만 분리된 로직을 페이지에 연결하는 것을 한 파일로 해결한 점이 아쉬워요. 이렇게 되면 로그인 페이지에서 nickname, password_check 관련 불필요한 코드를 불어오게 되니까요.
잘 분리하셨으니 login, signup.js 로 나눠서 작성하시면 각 페이지별 로직이 더 명확하게 표현될 것 같아요.
|
|
||
| const requireContent = (e) => { | ||
| console.log("requireContent called for:", e.target.id); | ||
| let content = e.target.value; |
There was a problem hiding this comment.
❗️ 수정요청
재할당하는 경우가 아니라면 const로 선언해주세요~
| let content = e.target.value; | |
| const content = e.target.value; |
| passwordIsNotSame: "비밀번호가 일치하지 않습니다.", | ||
| }; | ||
|
|
||
| const requireContent = (e) => { |
There was a problem hiding this comment.
💊 제안
해당 함수는 input의 value가 있으면 유효성 검사를 하고 없으면 에러 메시지를 보여주는 로직을 처리하고 있네요~
가독성 측면에서 내부 로직을 분리하면 더 좋을 것 같아요!
각 input 별로 유효성 검사를 하고 에러메시지를 보여주는 방식으로 분리하시면 좋겠네요~
const checkEmailValid = (input) => {
const emailRegex =
/^[a-zA-Z0-9]+([._-][a-zA-Z0-9]+)*@[a-zA-Z0-9-]+(\.[a-zA-Z]{2,})+$/;
if (emailRegex.test(input.value)) return ERRORMESSAGE.wrongEmail;
return ERRORMESSAGE.emailIsEmpty;
};
const checkPasswordValid = () => {
if (input.value.length < 8) return ERRORMESSAGE.wrongPassword;
return ERRORMESSAGE.passwordIsEmpty;
};
const checkNicknameValid = (input) => {
if (input.value.trim() === "") return ERRORMESSAGE.nicknameIsEmpty;
};
const requireContent = (e) => {
const target = e.target;
switch (target.id) {
case "email":
createNewMessage(checkEmailValid(target), target);
break;
case "password":
createNewMessage(checkPasswordValid(target), target);
document.querySelector(".visible_icon").style = "bottom: 6.9rem;";
break;
case "nickname":
createNewMessage(checkNicknameValid(target), target);
break;
}
if (e.target.nextElementSibling?.tagName == "P") {
e.target.style = "border: none";
e.target.nextElementSibling.remove();
document.querySelector(".visible_icon").style = "bottom: 1.4rem;";
}
};내부로직을 제가 다 잘 파악한 것인지 모르겠으니 위의 코드는 방향성만 참고해주세요~
|
|
||
| const visibleIconToggle = (e) => { | ||
| e.target.classList.toggle("passwordIsVisible"); | ||
| if (e.target.nextElementSibling.type == "password") { |
There was a problem hiding this comment.
💊 제안
이렇게 접근하시는 것도 나쁘지 않지만 이는 HTML 구조와 긴밀하게 연관되게 되므로,
HTML에서 위치만 변경되어도 작동되지 않을 위험이 커집니다~
가능하면 부모요소로 접근해 해당 요소 내에서 재탐색을 해보세요.
2 participants
요구사항
기본
로그인
회원가입
심화
주요 변경사항
스크린샷
멘토에게